Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(size): changed size in UnitOfWork #11749

Closed
wants to merge 1 commit into from

Conversation

spyrash
Copy link

@spyrash spyrash commented Dec 4, 2024

array_map and array_sum both iterate over the array.
A single loop could improve performance slightly.

array_map and array_sum both iterate over the array. A single loop could improve performance slightly.
@greg0ire
Copy link
Member

greg0ire commented Dec 4, 2024

Hi 👋 How did you find this issue?

@spyrash
Copy link
Author

spyrash commented Dec 4, 2024

Hi 👋 How did you find this issue?

Hi!
I was looking for similar improvements, as in !11332, and noticed how much the removal of a single array_map improved performance. Inspired by that, I started exploring other areas and realized that the size function could be optimized to use only one iteration of identityMap instead of two.
This refactor is not linked to any specific issue at the moment.

@greg0ire
Copy link
Member

greg0ire commented Dec 4, 2024

Ok. I was wondering because I was not able to find any call to that method in the project. Better performance comes at a legibility cost. For !11332, that cost was more than justified, but here, I don't think it is, by your own admission. Maybe a better approach would be to start by hunting for array_map in a real world trace?

@spyrash
Copy link
Author

spyrash commented Dec 5, 2024

The only reference I was able to find is in this docs->working-with-objects.rst. Despite the fact that this refactor remove an iteration and could be useful if other projects use this size function I agree with you.
I'll hunt for other improvements and try to get a benchmark/trace of it.

ps: there is any documentation of how to trace improvements and time of execution in doctrine for contribution?

@spyrash spyrash closed this Dec 5, 2024
@greg0ire
Copy link
Member

greg0ire commented Dec 5, 2024

I don't know if there is any docs on this, however I do know that we have phpbench benchmarks here: https://github.com/doctrine/orm/tree/3.3.x/tests/Performance

If I had to hunt for improvements, I think I would use xdebug, blackfire or datadog features and would hunt for calls to array_map in traces. With xdebug, you can get a full trace with xdebug_start_trace()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants